-
-
Notifications
You must be signed in to change notification settings - Fork 267
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Cite author in quotations and alt text #3008
Conversation
1b2d127
to
7cfdf23
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this! Just a small change requested
return text | ||
author = f"{name}: " if (name := self.author_text) else "" | ||
edition = f" ({info})" if (info := self.edition_info) else "" | ||
return f"{author}{self.title}{edition}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this case, I think this string should be localized, by wrapping it in _()
, since it is used in the UI
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I see what you mean, but in that case... isn't this string a bit useless in translation context, since it's fully formatted already? (with colons, e.g.—this format being what might need changes in translation).
What I mean is: to make this translatable, shouldn't we produce three un-collapsed translation strings instead?:
{author}: {title}
{title} ({edition})
{author}: {title} ({edition})
(At this point could also be {title} by {author} ({edition})
, but that's a separate matter.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah I see what you mean, since it's formatted in previous variables, translating the final string would be basically meaningless. It would be best from a translation standpoint to enumerate fully formed strings (even though that's annoying from a code standpoint), but I don't think that's a blocker to merging, since it already isn't translated
|
||
def save(self, *args: Any, **kwargs: Any) -> None: | ||
"""can't be abstract for query reasons, but you shouldn't USE it""" | ||
if not isinstance(self, Edition) and not isinstance(self, Work): | ||
if not isinstance(self, (Edition, Work)): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👌
No description provided.